Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvement for issue #178 in beeswithmachineguns, no helpful informa… #199

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

FliesLikeABrick
Copy link
Contributor

Improvement for #178 , no helpful information printed for _sting request failures, and is a fatal error. Modified to be non-fatal, prints error to stderr. This is not ideal, urllib.errors does not seem to exist on python2, was unable to find a way to catch HTTPError or URLERror specifically, so had to go with overly-broad exception catching

… information printed for _sting request failures, and is a fatal error. Modified to be non-fatal, prints error to stderr. This is not ideal, urllib.errors does not seem to exist on python2, was unable to find a way to catch HTTPError or URLERror specifically, so had to go with overly-broad exception catching
@cosmin
Copy link
Contributor

cosmin commented Nov 28, 2017

Perhaps look at switching to urllib3 instead? It’s the modern alternative and should make it easier to deal with this.

@FliesLikeABrick
Copy link
Contributor Author

@cosmin heh, I have been narrowly avoiding that since I wasn't sure about broaching the larger topic of python2 vs 3 support. I am happy to create a patch to change to urllib3 if we are ok with adding another non-stdlib dependency for python2. Thoughts?

@cosmin
Copy link
Contributor

cosmin commented Nov 29, 2017

I think that’s fine, better to add that dependency than deal with the urllib cluster

@FliesLikeABrick
Copy link
Contributor Author

ok, I will see work on this branch further (leaving the PR open) and take a shot at adapting to urllib3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants